Skip to content

Conversation

@afarber
Copy link
Contributor

@afarber afarber commented Jan 8, 2026

Issue

Partly fixes #7307

Run cucumber tests in parallel using GitHub Actions matrix.

The matrix creates 18 parallel jobs:

  • 3 build configs: clang-18-release, gcc-14-release, conan-linux-release
  • 2 algorithms: CH, MLD
  • 3 load methods: datastore, mmap, directly

Build artifacts are uploaded after compilation and downloaded by the test jobs. For Conan builds, runtime libraries are deployed using conan install --deployer=full_deploy so they can be included in the artifacts.

Tasklist

Requirements / Relations

Link any requirements here. Other pull requests this PR is based on?

@afarber afarber force-pushed the 7318-parallel-ch-mld branch 2 times, most recently from f918c35 to 7702559 Compare January 8, 2026 17:51
@afarber
Copy link
Contributor Author

afarber commented Jan 10, 2026

Hi @DennisOSRM and @TheMarex please review my PR -

Screenshot 2026-01-10 at 10 28 11

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request parallelizes cucumber tests by moving them from sequential execution within build jobs to a separate matrix of 18 parallel jobs (3 build configs × 2 algorithms × 3 load methods) using GitHub Actions matrix strategy. Build artifacts including binaries, libraries, and test data are uploaded after compilation and downloaded by the test jobs.

Changes:

  • Modified the build-matrix job to upload cucumber test artifacts for three specific build configurations
  • Added Conan runtime library deployment step for conan builds
  • Created a new cucumber-tests job that runs tests in parallel across multiple configurations
  • Updated ci-complete job to depend on the new cucumber-tests job

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File Description
CHANGELOG.md Added entry documenting the parallelization of cucumber tests using GitHub Actions matrix strategy
.github/workflows/osrm-backend.yml Replaced sequential cucumber test execution with parallel matrix job, added artifact upload/download steps, and configured runtime dependencies for test environments

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@MarcelloPerathoner
Copy link
Contributor

This PR should not be merged because:

  • it tests only 3 build variants, and 1 OS, whereas all variants should be tested,
  • while testing it still uses only 1 cpu per runner and there is a limit of 20 concurrent runners per repo, so it is using only 20 cpus (out of ~80 available)
  • testing will have to wait until the longest build is complete, meanwhile runners are idling, and it fails later,
  • it changes nothing for the developer at home, which still has to put up with terrible running times.

There is the better previous PR #7309 which:

  • will test all builds variants if configured to do so (linux, macOS and Windows) ,
  • uses all available CPUs,
  • starts testing early and fails fast,
  • delivers even more speed gains at home (total run time about 30s with 16 cpus).

@afarber
Copy link
Contributor Author

afarber commented Jan 10, 2026

Hi @MarcelloPerathoner this PR is not intended to replace your work in #7309.

The two pull requests are actually complementary: my PR parallelizes across configurations (different build configs, algorithms, load methods) using GitHub Actions matrix, while yours parallelizes within a single configuration using cucumber's --parallel flag.

In theory both could be combined - the matrix creates 18 jobs, and each could benefit from your --parallel optimization internally. But if the maintainers prefer to wait for #7309, I'm fine with closing this one.

@afarber afarber force-pushed the 7318-parallel-ch-mld branch from 708dc12 to 1fb6b97 Compare January 10, 2026 16:16
@afarber afarber requested a review from DennisOSRM January 10, 2026 18:40
@MarcelloPerathoner
Copy link
Contributor

Hi @MarcelloPerathoner this PR is not intended to replace your work in #7309.

The two pull requests are actually complementary: my PR parallelizes across configurations (different build configs, algorithms, load methods) using GitHub Actions matrix, while yours parallelizes within a single configuration using cucumber's --parallel flag.
In theory both could be combined - the matrix creates 18 jobs, and each could benefit from your --parallel optimization internally. But if the maintainers prefer to wait for #7309, I'm fine with closing this one.

Combining #7309 with this PR would only make things slower (and messier).

Take this data from an actual debug run and do the math:

Algorithm Load Method Passed Skipped Failed Elapsed (s)
ch mmap 1362 5 0 116.992
ch directly 1362 5 0 20.684
ch datastore 1354 13 0 20.646
mld mmap 1367 0 0 64.368
mld directly 1367 0 0 22.309
mld datastore 1359 8 0 24.745

After the debug build is done:

  • Parallel cucumbers #7309 will run those 6 tests on 4 CPUs. Total time (sum of the above): ~ 270s.
  • this PR will setup 6 new runners. The slowest test above will run about 4 times slower on a single CPU. Total time ~ 4*117s = 468s.

And that only if it can get a runner immediately. If you test more than 3 variants you are guaranteed not to get a runner immediately. If you test 15 variants as we currently do, you'll need 90 runners.

P.S. #7309 is ready for review.

@afarber
Copy link
Contributor Author

afarber commented Jan 10, 2026

  • ubuntu-latest runners have 2-4 vCPUs, not 1. The "4x slower on a single CPU" claim is incorrect.
  • With the matrix approach, all 6 configurations can run in parallel, so wall-clock time would be ~117s (the slowest config), not 468s
  • To combine both PRs, merge Parallel cucumbers #7309 first (for port/thread-safety), then change the cucumber-tests job to:
- name: Run cucumber tests
  run: OSRM_BUILD_DIR=... npx cucumber-js features/ -p ${{ matrix.algorithm }} --parallel 2

@MarcelloPerathoner
Copy link
Contributor

It runs faster only because you are testing 3 variants instead of 15. If you test more than 3 you will starve of runners and nothing is gained. Try testing 15 variants and report your times and we will see.

@afarber afarber force-pushed the 7318-parallel-ch-mld branch from 85d9c41 to e9c28ab Compare January 14, 2026 12:51
@afarber afarber marked this pull request as draft January 14, 2026 19:20
@afarber afarber force-pushed the 7318-parallel-ch-mld branch from e9c28ab to 0ef40d2 Compare January 14, 2026 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CI runs up from 0:45h to 4:30h

3 participants